Add ranged properties#452
Closed
JPBergsma wants to merge 35 commits into
Closed
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In This PR, I make a suggestion for how large “ranged” properties could be shared with OPTIMADE.
See the discussion in Issue#428
It also describes how this could be done for multidimensional properties.
I have now placed the main body of the text in section 3.9, I am however not sure if this is the best location. Feel free to suggest another location in the text.
I have now used a prefix to indicate that a property is a ranged property. This however creates a precedent, as it is the first time we use a prefix in this manner.If you prefer, I can also add a field that lists all the properties that are ranged properties, or mandate that this is specified at the info endpoint.Do we want to add a field in the property definition that a field is a ranged field, (also in case we choose to use a
prefix or aseparate property in the question before this one)?Now I have put a SHOULD level queryability not defined the queryability of all the dictionary fields except the values and indexes field.( I have not set all query ability to OPTIONAL)I do not think it is very important that these fields are queryable, so we could perhaps also make querying OPTIONAL.
Now the values can only depend linearly on the index. Do we want to define a more general field which could contain information on how to calculate the value from the index? This would be considerably more complex and probably warrants a PR of its own.Do we want to have a negative requirement on the range_id, i.e. If the range_id is not defined, a property must not have the same range as another property?For now, I removed properties average, min, max, and set as this becomes more complicated with multiple dimensions and I did not want to make this PR too big / complicated.
This can be addressed in a separate PR once this one gets merged.
Do we want to make the values for slicing optional?
The default values would then be [1,last,1]
So [] would return all the values, [100] would return all the values from the hundredth one onward and [30,50] would return every, value belonging to index 30 till 50.
Do we want to support multiple ranges for a single property? e.g. _ranged_property_name[1,100,2][101,200,1]